♻️ Refactor groups/classifiers and scicrunch to use asyncpg with service/repository separation
#8433
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #8433 +/- ##
==========================================
+ Coverage 87.11% 87.81% +0.70%
==========================================
Files 1961 1964 +3
Lines 76249 76327 +78
Branches 1342 1342
==========================================
+ Hits 66423 67026 +603
+ Misses 9422 8897 -525
Partials 404 404
Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
🧪 CI InsightsHere's what we observed from your CI run for af4d430. ✅ Passed Jobs With Interesting Signals
|
pcrespov
changed the title
groups/classifiers and scicrunch to use asyncpg with service/repository separation
pcrespov
added
a:webserver
…services and tests
…roupClassifierRepository
pcrespov
force-pushed
the
is4529/migrate-aiopg-to-asynpg-1
branch
from
dd3afe1 to
eca30ba
Compare
pcrespov
changed the title
groups/classifiers and scicrunch to use asyncpg with service/repository separationgroups/classifiers and scicrunch to use asyncpg with service/repository separation
pcrespov
requested review from
GitHK,
matusdrobuliak66 and
sanderegg
as code owners
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the groups/classifiers and scicrunch modules to migrate from aiopg to asyncpg while introducing a service/repository architectural pattern. The refactor improves separation of concerns by introducing dedicated repository classes for database operations and service layers for business logic.
Key changes include:
- Introduction of
GroupClassifierRepositoryandScicrunchResourcesRepositoryfor database operations - Creation of service layers (
GroupClassifiersServiceandScicrunchResourcesService) to handle business logic - Migration from
aiopgtoasyncpgwith updated exception handling (UniqueViolation→IntegrityError)
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
test_groups_classifiers_repository.py |
New test file for the group classifier repository functionality |
test_groups_classifiers.py |
Removed legacy test file that used aiopg patterns |
scicrunch_service.py |
New service module export for scicrunch functionality |
scicrunch/models.py |
Updated field validation using BeforeValidator instead of field_validator |
scicrunch/db.py |
Removed legacy database access layer using aiopg |
scicrunch/_service.py |
New service layer implementing business logic for scicrunch resources |
scicrunch/_repository.py |
New repository layer for scicrunch database operations using asyncpg |
_groups_repository.py |
Updated exception handling from UniqueViolation to IntegrityError |
_classifiers_service.py |
Refactored to use new repository pattern and service architecture |
_classifiers_rest.py |
Updated REST handlers to use service layers instead of direct repository access |
_classifiers_repository.py |
New repository for group classifier database operations |
exporter/_formatter/_sds.py |
Updated to use new scicrunch service instead of direct repository access |
faker_factories.py |
Added factory function for generating test group classifier data |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
services/web/server/tests/unit/with_dbs/01/test_groups_classifiers_repository.py
Outdated
Show resolved
Hide resolved
services/web/server/src/simcore_service_webserver/groups/_classifiers_repository.py
Outdated
Show resolved
Hide resolved
services/web/server/src/simcore_service_webserver/scicrunch/_service.py
Outdated
Show resolved
Hide resolved
wvangeit
approved these changes
Sep 29, 2025
giancarloromeo
approved these changes
Sep 29, 2025
|
@mergify queue |
🟠 Waiting for conditions to match
|
|
@pcrespov since you are on this topic, do you mind checking if the tests that check if the integration with scicrunch are still disabled and maybe restore them? Last time I looked they where disabled. |
…PKEY for improved service integration
pcrespov
force-pushed
the
is4529/migrate-aiopg-to-asynpg-1
branch
from
33a4728 to
4ef0000
Compare
|
This was referenced Oct 28, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What do these changes do?
This PR migrates database interactions in the
groups/classifiersandscicrunchmodules fromaiopgtoasyncpg. It introduces a major refactor aimed at clearer separation of concerns, improved repository patterns, and simplified service interfaces.Key changes include:
In Detail (by copilot)
Group Classifiers and Scicrunch Resource Refactor:
GroupClassifierRepositoryandScicrunchResourcesRepositoryclasses to encapsulate all database operations for group classifiers and Scicrunch resources, respectively, improving code maintainability and separation of concerns. [1] [2]GroupClassifiersService) to use the new repository, and moved logic for determining whether to use static bundles or dynamic RRID trees into this service layer. [1] [2]groups/_classifiers_rest.pyto use service classes (GroupClassifiersServiceandScicrunchResourcesService) instead of directly interacting with repositories or legacy service clients, simplifying handler logic and improving error handling. [1] [2] [3] [4] [5]UniqueViolationtoIntegrityErrorfor user-group operations. [1] [2] [3]ClassifierItemmodel.Minor improvements and dependency updates:
Related issue/s
How to test
Dev-ops
None